Skip to content

Conversation

Sirraide
Copy link
Member

This migrates a few dozen more AST visitors to use DRAV; for more information, see #115144. This also adds const in a number of places and removes around 30 uses of const_cast.

LLVM compiler time tracker link for this branch: https://llvm-compile-time-tracker.com/compare.php?from=ebcf1bf2ecba6b25ece3c2bbddb4485e76189387&to=a832fd0afe94c9ed3ac81544820919b49ee2f0ef&stat=instructions:u

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clangd clang-tidy clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. clang:static analyzer debuginfo HLSL HLSL Language Support labels Sep 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang-tools-extra

Author: None (Sirraide)

Changes

This migrates a few dozen more AST visitors to use DRAV; for more information, see #115144. This also adds const in a number of places and removes around 30 uses of const_cast.

LLVM compiler time tracker link for this branch: https://llvm-compile-time-tracker.com/compare.php?from=ebcf1bf2ecba6b25ece3c2bbddb4485e76189387&to=a832fd0afe94c9ed3ac81544820919b49ee2f0ef&stat=instructions:u


Patch is 206.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/160290.diff

54 Files Affected:

  • (modified) clang-tools-extra/clang-doc/Mapper.h (+11-11)
  • (modified) clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp (+8-11)
  • (modified) clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp (+9-9)
  • (modified) clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp (+7-7)
  • (modified) clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp (+26-25)
  • (modified) clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h (+37-53)
  • (modified) clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp (+4-7)
  • (modified) clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp (+11-11)
  • (modified) clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp (+6-4)
  • (modified) clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp (+19-14)
  • (modified) clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp (+13-13)
  • (modified) clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp (+5-5)
  • (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (+17-16)
  • (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp (+24-27)
  • (modified) clang-tools-extra/clangd/AST.cpp (+25-25)
  • (modified) clang-tools-extra/clangd/AST.h (+1-1)
  • (modified) clang-tools-extra/clangd/DumpAST.cpp (+36-30)
  • (modified) clang-tools-extra/clangd/FindTarget.cpp (+22-22)
  • (modified) clang-tools-extra/clangd/InlayHints.cpp (+27-27)
  • (modified) clang-tools-extra/clangd/SemanticHighlighting.cpp (+48-44)
  • (modified) clang-tools-extra/clangd/XRefs.cpp (+13-13)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp (+15-17)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp (+9-11)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/ASTTests.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/PrintASTTests.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/TestTU.cpp (+3-3)
  • (modified) clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp (+7-7)
  • (modified) clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp (+3-3)
  • (modified) clang-tools-extra/modularize/Modularize.cpp (+59-43)
  • (modified) clang-tools-extra/unittests/clang-doc/SerializeTest.cpp (+15-10)
  • (modified) clang-tools-extra/unittests/clang-tidy/OverlappingReplacementsTest.cpp (+4-4)
  • (modified) clang/include/clang/AST/DynamicRecursiveASTVisitor.h (+1-2)
  • (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+2)
  • (modified) clang/include/clang/InstallAPI/Visitor.h (+11-9)
  • (modified) clang/lib/AST/ASTImporterLookupTable.cpp (+11-10)
  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+10-11)
  • (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+4-4)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+10-11)
  • (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+25-25)
  • (modified) clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp (+7-7)
  • (modified) clang/lib/Frontend/ASTConsumers.cpp (+120-118)
  • (modified) clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp (+4-4)
  • (modified) clang/lib/Index/IndexBody.cpp (+46-45)
  • (modified) clang/lib/Index/IndexTypeSourceInfo.cpp (+21-20)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp (+14-15)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp (+16-17)
  • (modified) clang/lib/Tooling/ASTDiff/ASTDiff.cpp (+12-9)
  • (modified) clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp (+3-5)
  • (modified) clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp (+6-7)
  • (modified) clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp (+11-12)
  • (modified) clang/unittests/AST/RecursiveASTVisitorTest.cpp (+18-18)
  • (modified) clang/unittests/Frontend/NoAlterCodeGenActionTest.cpp (+4-4)
diff --git a/clang-tools-extra/clang-doc/Mapper.h b/clang-tools-extra/clang-doc/Mapper.h
index 322df6d594b3d..3d2e505515715 100644
--- a/clang-tools-extra/clang-doc/Mapper.h
+++ b/clang-tools-extra/clang-doc/Mapper.h
@@ -18,7 +18,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_DOC_MAPPER_H
 
 #include "Representation.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Tooling/Execution.h"
 
 using namespace clang::comments;
@@ -27,22 +27,22 @@ using namespace clang::tooling;
 namespace clang {
 namespace doc {
 
-class MapASTVisitor : public clang::RecursiveASTVisitor<MapASTVisitor>,
+class MapASTVisitor : public ConstDynamicRecursiveASTVisitor,
                       public ASTConsumer {
 public:
   explicit MapASTVisitor(ASTContext *Ctx, ClangDocContext CDCtx)
       : CDCtx(CDCtx) {}
 
   void HandleTranslationUnit(ASTContext &Context) override;
-  bool VisitNamespaceDecl(const NamespaceDecl *D);
-  bool VisitRecordDecl(const RecordDecl *D);
-  bool VisitEnumDecl(const EnumDecl *D);
-  bool VisitCXXMethodDecl(const CXXMethodDecl *D);
-  bool VisitFunctionDecl(const FunctionDecl *D);
-  bool VisitTypedefDecl(const TypedefDecl *D);
-  bool VisitTypeAliasDecl(const TypeAliasDecl *D);
-  bool VisitConceptDecl(const ConceptDecl *D);
-  bool VisitVarDecl(const VarDecl *D);
+  bool VisitNamespaceDecl(const NamespaceDecl *D) override;
+  bool VisitRecordDecl(const RecordDecl *D) override;
+  bool VisitEnumDecl(const EnumDecl *D) override;
+  bool VisitCXXMethodDecl(const CXXMethodDecl *D) override;
+  bool VisitFunctionDecl(const FunctionDecl *D) override;
+  bool VisitTypedefDecl(const TypedefDecl *D) override;
+  bool VisitTypeAliasDecl(const TypeAliasDecl *D) override;
+  bool VisitConceptDecl(const ConceptDecl *D) override;
+  bool VisitVarDecl(const VarDecl *D) override;
 
 private:
   template <typename T> bool mapDecl(const T *D, bool IsDefinition);
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
index 2c8856298e7be..66e34418fb0ac 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -8,7 +8,7 @@
 
 #include "AssignmentInIfConditionCheck.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
@@ -21,13 +21,13 @@ void AssignmentInIfConditionCheck::registerMatchers(MatchFinder *Finder) {
 
 void AssignmentInIfConditionCheck::check(
     const ast_matchers::MatchFinder::MatchResult &Result) {
-  class Visitor : public RecursiveASTVisitor<Visitor> {
+  class Visitor : public ConstDynamicRecursiveASTVisitor {
     AssignmentInIfConditionCheck &Check;
 
   public:
     explicit Visitor(AssignmentInIfConditionCheck &Check) : Check(Check) {}
-    bool VisitIfStmt(IfStmt *If) {
-      class ConditionVisitor : public RecursiveASTVisitor<ConditionVisitor> {
+    bool VisitIfStmt(const IfStmt *If) override {
+      class ConditionVisitor : public ConstDynamicRecursiveASTVisitor {
         AssignmentInIfConditionCheck &Check;
 
       public:
@@ -35,23 +35,20 @@ void AssignmentInIfConditionCheck::check(
             : Check(Check) {}
 
         // Dont traverse into any lambda expressions.
-        bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
-          return true;
-        }
+        bool TraverseLambdaExpr(const LambdaExpr *) override { return true; }
 
         // Dont traverse into any requires expressions.
-        bool TraverseRequiresExpr(RequiresExpr *,
-                                  DataRecursionQueue * = nullptr) {
+        bool TraverseRequiresExpr(const RequiresExpr *) override {
           return true;
         }
 
-        bool VisitBinaryOperator(BinaryOperator *BO) {
+        bool VisitBinaryOperator(const BinaryOperator *BO) override {
           if (BO->isAssignmentOp())
             Check.report(BO);
           return true;
         }
 
-        bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *OCE) {
+        bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) override {
           if (OCE->isAssignmentOp())
             Check.report(OCE);
           return true;
diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
index d8207b30f1b5e..1a677b512320c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -9,7 +9,7 @@
 #include "EasilySwappableParametersCheck.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/SmallSet.h"
@@ -1600,8 +1600,8 @@ static bool lazyMapOfSetsIntersectionExists(const MapTy &Map, const ElemTy &E1,
 /// a usage for both in the same strict expression subtree. A strict
 /// expression subtree is a tree which only includes Expr nodes, i.e. no
 /// Stmts and no Decls.
-class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
-  using Base = RecursiveASTVisitor<AppearsInSameExpr>;
+class AppearsInSameExpr : public ConstDynamicRecursiveASTVisitor {
+  using Base = ConstDynamicRecursiveASTVisitor;
 
   const FunctionDecl *FD;
   const Expr *CurrentExprOnlyTreeRoot = nullptr;
@@ -1612,7 +1612,7 @@ class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
 public:
   void setup(const FunctionDecl *FD) {
     this->FD = FD;
-    TraverseFunctionDecl(const_cast<FunctionDecl *>(FD));
+    TraverseFunctionDecl(FD);
   }
 
   bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
@@ -1620,13 +1620,13 @@ class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
                                            Param2);
   }
 
-  bool TraverseDecl(Decl *D) {
+  bool TraverseDecl(const Decl *D) override {
     CurrentExprOnlyTreeRoot = nullptr;
     return Base::TraverseDecl(D);
   }
 
-  bool TraverseStmt(Stmt *S, DataRecursionQueue *Queue = nullptr) {
-    if (auto *E = dyn_cast_or_null<Expr>(S)) {
+  bool TraverseStmt(const Stmt *S) override {
+    if (const auto *E = dyn_cast_or_null<Expr>(S)) {
       bool RootSetInCurrentStackFrame = false;
       if (!CurrentExprOnlyTreeRoot) {
         CurrentExprOnlyTreeRoot = E;
@@ -1646,11 +1646,11 @@ class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
     return Base::TraverseStmt(S);
   }
 
-  bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+  bool VisitDeclRefExpr(const DeclRefExpr *DRE) override {
     if (!CurrentExprOnlyTreeRoot)
       return true;
 
-    if (auto *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl()))
+    if (const auto *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl()))
       if (llvm::find(FD->parameters(), PVD))
         ParentExprsForParamRefs[PVD].insert(CurrentExprOnlyTreeRoot);
 
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
index 9f4c215614287..47b592842df5b 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DeprecatedHeadersCheck.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
@@ -44,19 +44,19 @@ class IncludeModernizePPCallbacks : public PPCallbacks {
   bool CheckHeaderFile;
 };
 
-class ExternCRefutationVisitor
-    : public RecursiveASTVisitor<ExternCRefutationVisitor> {
+class ExternCRefutationVisitor : public ConstDynamicRecursiveASTVisitor {
   std::vector<IncludeMarker> &IncludesToBeProcessed;
   const SourceManager &SM;
 
 public:
   ExternCRefutationVisitor(std::vector<IncludeMarker> &IncludesToBeProcessed,
                            SourceManager &SM)
-      : IncludesToBeProcessed(IncludesToBeProcessed), SM(SM) {}
-  bool shouldWalkTypesOfTypeLocs() const { return false; }
-  bool shouldVisitLambdaBody() const { return false; }
+      : IncludesToBeProcessed(IncludesToBeProcessed), SM(SM) {
+    ShouldWalkTypesOfTypeLocs = false;
+    ShouldVisitLambdaBody = false;
+  }
 
-  bool VisitLinkageSpecDecl(LinkageSpecDecl *LinkSpecDecl) const {
+  bool VisitLinkageSpecDecl(const LinkageSpecDecl *LinkSpecDecl) override {
     if (LinkSpecDecl->getLanguage() != LinkageSpecLanguageIDs::C ||
         !LinkSpecDecl->hasBraces())
       return true;
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index 286c39be44ce4..f9de75656a0d9 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -34,10 +34,10 @@ namespace clang::tidy::modernize {
 /// RecursiveASTVisitor::TraverseStmt() and pop_back() afterwards. The Stmt atop
 /// the stack is the parent of the current statement (NULL for the topmost
 /// statement).
-bool StmtAncestorASTVisitor::TraverseStmt(Stmt *Statement) {
+bool StmtAncestorASTVisitor::TraverseStmt(const Stmt *Statement) {
   StmtAncestors.insert(std::make_pair(Statement, StmtStack.back()));
   StmtStack.push_back(Statement);
-  RecursiveASTVisitor<StmtAncestorASTVisitor>::TraverseStmt(Statement);
+  ConstDynamicRecursiveASTVisitor::TraverseStmt(Statement);
   StmtStack.pop_back();
   return true;
 }
@@ -47,7 +47,7 @@ bool StmtAncestorASTVisitor::TraverseStmt(Stmt *Statement) {
 /// Combined with StmtAncestors, this provides roughly the same information as
 /// Scope, as we can map a VarDecl to its DeclStmt, then walk up the parent tree
 /// using StmtAncestors.
-bool StmtAncestorASTVisitor::VisitDeclStmt(DeclStmt *Statement) {
+bool StmtAncestorASTVisitor::VisitDeclStmt(const DeclStmt *Statement) {
   for (const auto *Decl : Statement->decls()) {
     if (const auto *V = dyn_cast<VarDecl>(Decl))
       DeclParents.insert(std::make_pair(V, Statement));
@@ -56,27 +56,27 @@ bool StmtAncestorASTVisitor::VisitDeclStmt(DeclStmt *Statement) {
 }
 
 /// record the DeclRefExpr as part of the parent expression.
-bool ComponentFinderASTVisitor::VisitDeclRefExpr(DeclRefExpr *E) {
+bool ComponentFinderASTVisitor::VisitDeclRefExpr(const DeclRefExpr *E) {
   Components.push_back(E);
   return true;
 }
 
 /// record the MemberExpr as part of the parent expression.
-bool ComponentFinderASTVisitor::VisitMemberExpr(MemberExpr *Member) {
+bool ComponentFinderASTVisitor::VisitMemberExpr(const MemberExpr *Member) {
   Components.push_back(Member);
   return true;
 }
 
 /// Forward any DeclRefExprs to a check on the referenced variable
 /// declaration.
-bool DependencyFinderASTVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {
+bool DependencyFinderASTVisitor::VisitDeclRefExpr(const DeclRefExpr *DeclRef) {
   if (auto *V = dyn_cast_or_null<VarDecl>(DeclRef->getDecl()))
     return VisitVarDecl(V);
   return true;
 }
 
 /// Determine if any this variable is declared inside the ContainingStmt.
-bool DependencyFinderASTVisitor::VisitVarDecl(VarDecl *V) {
+bool DependencyFinderASTVisitor::VisitVarDecl(const VarDecl *V) {
   const Stmt *Curr = DeclParents->lookup(V);
   // First, see if the variable was declared within an inner scope of the loop.
   while (Curr != nullptr) {
@@ -100,7 +100,7 @@ bool DependencyFinderASTVisitor::VisitVarDecl(VarDecl *V) {
 
 /// If we already created a variable for TheLoop, check to make sure
 /// that the name was not already taken.
-bool DeclFinderASTVisitor::VisitForStmt(ForStmt *TheLoop) {
+bool DeclFinderASTVisitor::VisitForStmt(const ForStmt *TheLoop) {
   StmtGeneratedVarNameMap::const_iterator I = GeneratedDecls->find(TheLoop);
   if (I != GeneratedDecls->end() && I->second == Name) {
     Found = true;
@@ -111,7 +111,7 @@ bool DeclFinderASTVisitor::VisitForStmt(ForStmt *TheLoop) {
 
 /// If any named declaration within the AST subtree has the same name,
 /// then consider Name already taken.
-bool DeclFinderASTVisitor::VisitNamedDecl(NamedDecl *D) {
+bool DeclFinderASTVisitor::VisitNamedDecl(const NamedDecl *D) {
   const IdentifierInfo *Ident = D->getIdentifier();
   if (Ident && Ident->getName() == Name) {
     Found = true;
@@ -122,7 +122,7 @@ bool DeclFinderASTVisitor::VisitNamedDecl(NamedDecl *D) {
 
 /// Forward any declaration references to the actual check on the
 /// referenced declaration.
-bool DeclFinderASTVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {
+bool DeclFinderASTVisitor::VisitDeclRefExpr(const DeclRefExpr *DeclRef) {
   if (auto *D = dyn_cast<NamedDecl>(DeclRef->getDecl()))
     return VisitNamedDecl(D);
   return true;
@@ -497,7 +497,7 @@ void ForLoopIndexUseVisitor::addUsage(const Usage &U) {
 ///     int k = *i + 2;
 ///   }
 /// \endcode
-bool ForLoopIndexUseVisitor::TraverseUnaryOperator(UnaryOperator *Uop) {
+bool ForLoopIndexUseVisitor::TraverseUnaryOperator(const UnaryOperator *Uop) {
   // If we dereference an iterator that's actually a pointer, count the
   // occurrence.
   if (isDereferenceOfUop(Uop, IndexVar)) {
@@ -533,7 +533,7 @@ bool ForLoopIndexUseVisitor::TraverseUnaryOperator(UnaryOperator *Uop) {
 ///   }
 /// \endcode
 /// will not.
-bool ForLoopIndexUseVisitor::TraverseMemberExpr(MemberExpr *Member) {
+bool ForLoopIndexUseVisitor::TraverseMemberExpr(const MemberExpr *Member) {
   const Expr *Base = Member->getBase();
   const DeclRefExpr *Obj = getDeclRef(Base);
   const Expr *ResultExpr = Member;
@@ -593,8 +593,8 @@ bool ForLoopIndexUseVisitor::TraverseMemberExpr(MemberExpr *Member) {
 /// Calls on the iterator object are not permitted, unless done through
 /// operator->(). The one exception is allowing vector::at() for pseudoarrays.
 bool ForLoopIndexUseVisitor::TraverseCXXMemberCallExpr(
-    CXXMemberCallExpr *MemberCall) {
-  auto *Member =
+    const CXXMemberCallExpr *MemberCall) {
+  const auto *Member =
       dyn_cast<MemberExpr>(MemberCall->getCallee()->IgnoreParenImpCasts());
   if (!Member)
     return VisitorBase::TraverseCXXMemberCallExpr(MemberCall);
@@ -638,7 +638,7 @@ bool ForLoopIndexUseVisitor::TraverseCXXMemberCallExpr(
 ///   }
 /// \endcode
 bool ForLoopIndexUseVisitor::TraverseCXXOperatorCallExpr(
-    CXXOperatorCallExpr *OpCall) {
+    const CXXOperatorCallExpr *OpCall) {
   switch (OpCall->getOperator()) {
   case OO_Star:
     if (isDereferenceOfOpCall(OpCall, IndexVar)) {
@@ -683,8 +683,9 @@ bool ForLoopIndexUseVisitor::TraverseCXXOperatorCallExpr(
 /// \endcode
 /// and further checking needs to be done later to ensure that exactly one array
 /// is referenced.
-bool ForLoopIndexUseVisitor::TraverseArraySubscriptExpr(ArraySubscriptExpr *E) {
-  Expr *Arr = E->getBase();
+bool ForLoopIndexUseVisitor::TraverseArraySubscriptExpr(
+    const ArraySubscriptExpr *E) {
+  const Expr *Arr = E->getBase();
   if (!isIndexInSubscriptExpr(E->getIdx(), IndexVar))
     return VisitorBase::TraverseArraySubscriptExpr(E);
 
@@ -737,7 +738,7 @@ bool ForLoopIndexUseVisitor::TraverseArraySubscriptExpr(ArraySubscriptExpr *E) {
 ///    for (int i = 0; i < obj.getVector().size(); ++i)
 ///      obj.foo(10); // using `obj` is considered risky
 /// \endcode
-bool ForLoopIndexUseVisitor::VisitDeclRefExpr(DeclRefExpr *E) {
+bool ForLoopIndexUseVisitor::VisitDeclRefExpr(const DeclRefExpr *E) {
   const ValueDecl *TheDecl = E->getDecl();
   if (areSameVariable(IndexVar, TheDecl) ||
       exprReferencesVariable(IndexVar, E) || areSameVariable(EndVar, TheDecl) ||
@@ -770,9 +771,9 @@ bool ForLoopIndexUseVisitor::VisitDeclRefExpr(DeclRefExpr *E) {
 ///     f(elem);
 ///   }
 /// \endcode
-bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
+bool ForLoopIndexUseVisitor::TraverseLambdaCapture(const LambdaExpr *LE,
                                                    const LambdaCapture *C,
-                                                   Expr *Init) {
+                                                   const Expr *Init) {
   if (C->capturesVariable()) {
     ValueDecl *VDecl = C->getCapturedVar();
     if (areSameVariable(IndexVar, VDecl)) {
@@ -785,7 +786,7 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
                      C->getLocation()));
     }
     if (VDecl->isInitCapture())
-      TraverseStmtImpl(cast<VarDecl>(VDecl)->getInit());
+      traverseStmtImpl(cast<VarDecl>(VDecl)->getInit());
   }
   return VisitorBase::TraverseLambdaCapture(LE, C, Init);
 }
@@ -794,7 +795,7 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
 /// element, note it for reuse as the loop variable.
 ///
 /// See the comments for isAliasDecl.
-bool ForLoopIndexUseVisitor::VisitDeclStmt(DeclStmt *S) {
+bool ForLoopIndexUseVisitor::VisitDeclStmt(const DeclStmt *S) {
   if (!AliasDecl && S->isSingleDecl() &&
       isAliasDecl(Context, S->getSingleDecl(), IndexVar)) {
     AliasDecl = S;
@@ -815,7 +816,7 @@ bool ForLoopIndexUseVisitor::VisitDeclStmt(DeclStmt *S) {
   return true;
 }
 
-bool ForLoopIndexUseVisitor::TraverseStmtImpl(Stmt *S) {
+bool ForLoopIndexUseVisitor::traverseStmtImpl(const Stmt *S) {
   // All this pointer swapping is a mechanism for tracking immediate parentage
   // of Stmts.
   const Stmt *OldNextParent = NextStmtParent;
@@ -826,7 +827,7 @@ bool ForLoopIndexUseVisitor::TraverseStmtImpl(Stmt *S) {
   return Result;
 }
 
-bool ForLoopIndexUseVisitor::TraverseStmt(Stmt *S) {
+bool ForLoopIndexUseVisitor::TraverseStmt(const Stmt *S) {
   // If this is an initialization expression for a lambda capture, prune the
   // traversal so that we don't end up diagnosing the contained DeclRefExpr as
   // inconsistent usage. No need to record the usage here -- this is done in
@@ -838,7 +839,7 @@ bool ForLoopIndexUseVisitor::TraverseStmt(Stmt *S) {
       return true;
     }
   }
-  return TraverseStmtImpl(S);
+  return traverseStmtImpl(S);
 }
 
 std::string VariableNamer::createIndexName() {
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index 306eca7140d1a..dc7ab847d9921 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_LOOP_CONVERT_UTILS_H
 
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseMap.h"
@@ -53,8 +53,7 @@ using ComponentVector = llvm::SmallVector<const clang::Expr *, 16>;
 
 /// Class used build the reverse AST properties needed to detect
 /// name conflicts and free variables.
-class StmtAncestorASTVisitor
-    : public clang::RecursiveASTVisitor<StmtAncestorASTVisitor> {
+class StmtAncestorASTVisitor : public ConstDynamicRecursiveASTVisitor {
 public:
   StmtAncestorASTVisitor() { StmtStack.push_back(nullptr); }
 
@@ -72,45 +71,37 @@ class StmtAncestorASTVisitor
   /// Accessor for DeclParents.
   const DeclParentMap &getDeclToParentStmtMap() { return DeclParents; }
 
-  friend class clang::RecursiveASTVisitor<StmtAncestorASTVisitor>;
-
 private:
   StmtParentMap StmtAncestors;
   DeclParentMap DeclParents;
   llvm::SmallVector<const clang::Stmt *, 16> StmtStack;
 
-  bool TraverseStmt(clang::Stmt *Statement);
-  bool VisitDeclStmt(clang::DeclStmt *Statement);
+  bool TraverseStmt(const Stmt *Statement) override;
+  bool VisitDeclStmt(const DeclStmt *Statement) override;
 };
 
 /// Class used to find the variables and member expressions on which an
 /// arbitrary expression depends.
-class ComponentFinderASTVisitor
-    : public clang::RecursiveASTVisitor<ComponentFinderASTVisitor> {
+class ComponentFinderASTVisitor : public ConstDynamicRecursiveASTVisitor {
 public:
   ComponentFinderASTVisitor() = default;
 
   /// Find the...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-clang-codegen

Author: None (Sirraide)

Changes

This migrates a few dozen more AST visitors to use DRAV; for more information, see #115144. This also adds const in a number of places and removes around 30 uses of const_cast.

LLVM compiler time tracker link for this branch: https://llvm-compile-time-tracker.com/compare.php?from=ebcf1bf2ecba6b25ece3c2bbddb4485e76189387&amp;to=a832fd0afe94c9ed3ac81544820919b49ee2f0ef&amp;stat=instructions:u


Patch is 206.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/160290.diff

54 Files Affected:

  • (modified) clang-tools-extra/clang-doc/Mapper.h (+11-11)
  • (modified) clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp (+8-11)
  • (modified) clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp (+9-9)
  • (modified) clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp (+7-7)
  • (modified) clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp (+26-25)
  • (modified) clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h (+37-53)
  • (modified) clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp (+4-7)
  • (modified) clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp (+11-11)
  • (modified) clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp (+6-4)
  • (modified) clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp (+19-14)
  • (modified) clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp (+13-13)
  • (modified) clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp (+5-5)
  • (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (+17-16)
  • (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp (+24-27)
  • (modified) clang-tools-extra/clangd/AST.cpp (+25-25)
  • (modified) clang-tools-extra/clangd/AST.h (+1-1)
  • (modified) clang-tools-extra/clangd/DumpAST.cpp (+36-30)
  • (modified) clang-tools-extra/clangd/FindTarget.cpp (+22-22)
  • (modified) clang-tools-extra/clangd/InlayHints.cpp (+27-27)
  • (modified) clang-tools-extra/clangd/SemanticHighlighting.cpp (+48-44)
  • (modified) clang-tools-extra/clangd/XRefs.cpp (+13-13)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp (+15-17)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp (+9-11)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/ASTTests.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/PrintASTTests.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/TestTU.cpp (+3-3)
  • (modified) clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp (+7-7)
  • (modified) clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp (+3-3)
  • (modified) clang-tools-extra/modularize/Modularize.cpp (+59-43)
  • (modified) clang-tools-extra/unittests/clang-doc/SerializeTest.cpp (+15-10)
  • (modified) clang-tools-extra/unittests/clang-tidy/OverlappingReplacementsTest.cpp (+4-4)
  • (modified) clang/include/clang/AST/DynamicRecursiveASTVisitor.h (+1-2)
  • (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+2)
  • (modified) clang/include/clang/InstallAPI/Visitor.h (+11-9)
  • (modified) clang/lib/AST/ASTImporterLookupTable.cpp (+11-10)
  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+10-11)
  • (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+4-4)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+10-11)
  • (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+25-25)
  • (modified) clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp (+7-7)
  • (modified) clang/lib/Frontend/ASTConsumers.cpp (+120-118)
  • (modified) clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp (+4-4)
  • (modified) clang/lib/Index/IndexBody.cpp (+46-45)
  • (modified) clang/lib/Index/IndexTypeSourceInfo.cpp (+21-20)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp (+14-15)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp (+16-17)
  • (modified) clang/lib/Tooling/ASTDiff/ASTDiff.cpp (+12-9)
  • (modified) clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp (+3-5)
  • (modified) clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp (+6-7)
  • (modified) clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp (+11-12)
  • (modified) clang/unittests/AST/RecursiveASTVisitorTest.cpp (+18-18)
  • (modified) clang/unittests/Frontend/NoAlterCodeGenActionTest.cpp (+4-4)
diff --git a/clang-tools-extra/clang-doc/Mapper.h b/clang-tools-extra/clang-doc/Mapper.h
index 322df6d594b3d..3d2e505515715 100644
--- a/clang-tools-extra/clang-doc/Mapper.h
+++ b/clang-tools-extra/clang-doc/Mapper.h
@@ -18,7 +18,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_DOC_MAPPER_H
 
 #include "Representation.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Tooling/Execution.h"
 
 using namespace clang::comments;
@@ -27,22 +27,22 @@ using namespace clang::tooling;
 namespace clang {
 namespace doc {
 
-class MapASTVisitor : public clang::RecursiveASTVisitor<MapASTVisitor>,
+class MapASTVisitor : public ConstDynamicRecursiveASTVisitor,
                       public ASTConsumer {
 public:
   explicit MapASTVisitor(ASTContext *Ctx, ClangDocContext CDCtx)
       : CDCtx(CDCtx) {}
 
   void HandleTranslationUnit(ASTContext &Context) override;
-  bool VisitNamespaceDecl(const NamespaceDecl *D);
-  bool VisitRecordDecl(const RecordDecl *D);
-  bool VisitEnumDecl(const EnumDecl *D);
-  bool VisitCXXMethodDecl(const CXXMethodDecl *D);
-  bool VisitFunctionDecl(const FunctionDecl *D);
-  bool VisitTypedefDecl(const TypedefDecl *D);
-  bool VisitTypeAliasDecl(const TypeAliasDecl *D);
-  bool VisitConceptDecl(const ConceptDecl *D);
-  bool VisitVarDecl(const VarDecl *D);
+  bool VisitNamespaceDecl(const NamespaceDecl *D) override;
+  bool VisitRecordDecl(const RecordDecl *D) override;
+  bool VisitEnumDecl(const EnumDecl *D) override;
+  bool VisitCXXMethodDecl(const CXXMethodDecl *D) override;
+  bool VisitFunctionDecl(const FunctionDecl *D) override;
+  bool VisitTypedefDecl(const TypedefDecl *D) override;
+  bool VisitTypeAliasDecl(const TypeAliasDecl *D) override;
+  bool VisitConceptDecl(const ConceptDecl *D) override;
+  bool VisitVarDecl(const VarDecl *D) override;
 
 private:
   template <typename T> bool mapDecl(const T *D, bool IsDefinition);
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
index 2c8856298e7be..66e34418fb0ac 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -8,7 +8,7 @@
 
 #include "AssignmentInIfConditionCheck.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
@@ -21,13 +21,13 @@ void AssignmentInIfConditionCheck::registerMatchers(MatchFinder *Finder) {
 
 void AssignmentInIfConditionCheck::check(
     const ast_matchers::MatchFinder::MatchResult &Result) {
-  class Visitor : public RecursiveASTVisitor<Visitor> {
+  class Visitor : public ConstDynamicRecursiveASTVisitor {
     AssignmentInIfConditionCheck &Check;
 
   public:
     explicit Visitor(AssignmentInIfConditionCheck &Check) : Check(Check) {}
-    bool VisitIfStmt(IfStmt *If) {
-      class ConditionVisitor : public RecursiveASTVisitor<ConditionVisitor> {
+    bool VisitIfStmt(const IfStmt *If) override {
+      class ConditionVisitor : public ConstDynamicRecursiveASTVisitor {
         AssignmentInIfConditionCheck &Check;
 
       public:
@@ -35,23 +35,20 @@ void AssignmentInIfConditionCheck::check(
             : Check(Check) {}
 
         // Dont traverse into any lambda expressions.
-        bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
-          return true;
-        }
+        bool TraverseLambdaExpr(const LambdaExpr *) override { return true; }
 
         // Dont traverse into any requires expressions.
-        bool TraverseRequiresExpr(RequiresExpr *,
-                                  DataRecursionQueue * = nullptr) {
+        bool TraverseRequiresExpr(const RequiresExpr *) override {
           return true;
         }
 
-        bool VisitBinaryOperator(BinaryOperator *BO) {
+        bool VisitBinaryOperator(const BinaryOperator *BO) override {
           if (BO->isAssignmentOp())
             Check.report(BO);
           return true;
         }
 
-        bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *OCE) {
+        bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) override {
           if (OCE->isAssignmentOp())
             Check.report(OCE);
           return true;
diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
index d8207b30f1b5e..1a677b512320c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -9,7 +9,7 @@
 #include "EasilySwappableParametersCheck.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/SmallSet.h"
@@ -1600,8 +1600,8 @@ static bool lazyMapOfSetsIntersectionExists(const MapTy &Map, const ElemTy &E1,
 /// a usage for both in the same strict expression subtree. A strict
 /// expression subtree is a tree which only includes Expr nodes, i.e. no
 /// Stmts and no Decls.
-class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
-  using Base = RecursiveASTVisitor<AppearsInSameExpr>;
+class AppearsInSameExpr : public ConstDynamicRecursiveASTVisitor {
+  using Base = ConstDynamicRecursiveASTVisitor;
 
   const FunctionDecl *FD;
   const Expr *CurrentExprOnlyTreeRoot = nullptr;
@@ -1612,7 +1612,7 @@ class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
 public:
   void setup(const FunctionDecl *FD) {
     this->FD = FD;
-    TraverseFunctionDecl(const_cast<FunctionDecl *>(FD));
+    TraverseFunctionDecl(FD);
   }
 
   bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
@@ -1620,13 +1620,13 @@ class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
                                            Param2);
   }
 
-  bool TraverseDecl(Decl *D) {
+  bool TraverseDecl(const Decl *D) override {
     CurrentExprOnlyTreeRoot = nullptr;
     return Base::TraverseDecl(D);
   }
 
-  bool TraverseStmt(Stmt *S, DataRecursionQueue *Queue = nullptr) {
-    if (auto *E = dyn_cast_or_null<Expr>(S)) {
+  bool TraverseStmt(const Stmt *S) override {
+    if (const auto *E = dyn_cast_or_null<Expr>(S)) {
       bool RootSetInCurrentStackFrame = false;
       if (!CurrentExprOnlyTreeRoot) {
         CurrentExprOnlyTreeRoot = E;
@@ -1646,11 +1646,11 @@ class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
     return Base::TraverseStmt(S);
   }
 
-  bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+  bool VisitDeclRefExpr(const DeclRefExpr *DRE) override {
     if (!CurrentExprOnlyTreeRoot)
       return true;
 
-    if (auto *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl()))
+    if (const auto *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl()))
       if (llvm::find(FD->parameters(), PVD))
         ParentExprsForParamRefs[PVD].insert(CurrentExprOnlyTreeRoot);
 
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
index 9f4c215614287..47b592842df5b 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DeprecatedHeadersCheck.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
@@ -44,19 +44,19 @@ class IncludeModernizePPCallbacks : public PPCallbacks {
   bool CheckHeaderFile;
 };
 
-class ExternCRefutationVisitor
-    : public RecursiveASTVisitor<ExternCRefutationVisitor> {
+class ExternCRefutationVisitor : public ConstDynamicRecursiveASTVisitor {
   std::vector<IncludeMarker> &IncludesToBeProcessed;
   const SourceManager &SM;
 
 public:
   ExternCRefutationVisitor(std::vector<IncludeMarker> &IncludesToBeProcessed,
                            SourceManager &SM)
-      : IncludesToBeProcessed(IncludesToBeProcessed), SM(SM) {}
-  bool shouldWalkTypesOfTypeLocs() const { return false; }
-  bool shouldVisitLambdaBody() const { return false; }
+      : IncludesToBeProcessed(IncludesToBeProcessed), SM(SM) {
+    ShouldWalkTypesOfTypeLocs = false;
+    ShouldVisitLambdaBody = false;
+  }
 
-  bool VisitLinkageSpecDecl(LinkageSpecDecl *LinkSpecDecl) const {
+  bool VisitLinkageSpecDecl(const LinkageSpecDecl *LinkSpecDecl) override {
     if (LinkSpecDecl->getLanguage() != LinkageSpecLanguageIDs::C ||
         !LinkSpecDecl->hasBraces())
       return true;
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index 286c39be44ce4..f9de75656a0d9 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -34,10 +34,10 @@ namespace clang::tidy::modernize {
 /// RecursiveASTVisitor::TraverseStmt() and pop_back() afterwards. The Stmt atop
 /// the stack is the parent of the current statement (NULL for the topmost
 /// statement).
-bool StmtAncestorASTVisitor::TraverseStmt(Stmt *Statement) {
+bool StmtAncestorASTVisitor::TraverseStmt(const Stmt *Statement) {
   StmtAncestors.insert(std::make_pair(Statement, StmtStack.back()));
   StmtStack.push_back(Statement);
-  RecursiveASTVisitor<StmtAncestorASTVisitor>::TraverseStmt(Statement);
+  ConstDynamicRecursiveASTVisitor::TraverseStmt(Statement);
   StmtStack.pop_back();
   return true;
 }
@@ -47,7 +47,7 @@ bool StmtAncestorASTVisitor::TraverseStmt(Stmt *Statement) {
 /// Combined with StmtAncestors, this provides roughly the same information as
 /// Scope, as we can map a VarDecl to its DeclStmt, then walk up the parent tree
 /// using StmtAncestors.
-bool StmtAncestorASTVisitor::VisitDeclStmt(DeclStmt *Statement) {
+bool StmtAncestorASTVisitor::VisitDeclStmt(const DeclStmt *Statement) {
   for (const auto *Decl : Statement->decls()) {
     if (const auto *V = dyn_cast<VarDecl>(Decl))
       DeclParents.insert(std::make_pair(V, Statement));
@@ -56,27 +56,27 @@ bool StmtAncestorASTVisitor::VisitDeclStmt(DeclStmt *Statement) {
 }
 
 /// record the DeclRefExpr as part of the parent expression.
-bool ComponentFinderASTVisitor::VisitDeclRefExpr(DeclRefExpr *E) {
+bool ComponentFinderASTVisitor::VisitDeclRefExpr(const DeclRefExpr *E) {
   Components.push_back(E);
   return true;
 }
 
 /// record the MemberExpr as part of the parent expression.
-bool ComponentFinderASTVisitor::VisitMemberExpr(MemberExpr *Member) {
+bool ComponentFinderASTVisitor::VisitMemberExpr(const MemberExpr *Member) {
   Components.push_back(Member);
   return true;
 }
 
 /// Forward any DeclRefExprs to a check on the referenced variable
 /// declaration.
-bool DependencyFinderASTVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {
+bool DependencyFinderASTVisitor::VisitDeclRefExpr(const DeclRefExpr *DeclRef) {
   if (auto *V = dyn_cast_or_null<VarDecl>(DeclRef->getDecl()))
     return VisitVarDecl(V);
   return true;
 }
 
 /// Determine if any this variable is declared inside the ContainingStmt.
-bool DependencyFinderASTVisitor::VisitVarDecl(VarDecl *V) {
+bool DependencyFinderASTVisitor::VisitVarDecl(const VarDecl *V) {
   const Stmt *Curr = DeclParents->lookup(V);
   // First, see if the variable was declared within an inner scope of the loop.
   while (Curr != nullptr) {
@@ -100,7 +100,7 @@ bool DependencyFinderASTVisitor::VisitVarDecl(VarDecl *V) {
 
 /// If we already created a variable for TheLoop, check to make sure
 /// that the name was not already taken.
-bool DeclFinderASTVisitor::VisitForStmt(ForStmt *TheLoop) {
+bool DeclFinderASTVisitor::VisitForStmt(const ForStmt *TheLoop) {
   StmtGeneratedVarNameMap::const_iterator I = GeneratedDecls->find(TheLoop);
   if (I != GeneratedDecls->end() && I->second == Name) {
     Found = true;
@@ -111,7 +111,7 @@ bool DeclFinderASTVisitor::VisitForStmt(ForStmt *TheLoop) {
 
 /// If any named declaration within the AST subtree has the same name,
 /// then consider Name already taken.
-bool DeclFinderASTVisitor::VisitNamedDecl(NamedDecl *D) {
+bool DeclFinderASTVisitor::VisitNamedDecl(const NamedDecl *D) {
   const IdentifierInfo *Ident = D->getIdentifier();
   if (Ident && Ident->getName() == Name) {
     Found = true;
@@ -122,7 +122,7 @@ bool DeclFinderASTVisitor::VisitNamedDecl(NamedDecl *D) {
 
 /// Forward any declaration references to the actual check on the
 /// referenced declaration.
-bool DeclFinderASTVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {
+bool DeclFinderASTVisitor::VisitDeclRefExpr(const DeclRefExpr *DeclRef) {
   if (auto *D = dyn_cast<NamedDecl>(DeclRef->getDecl()))
     return VisitNamedDecl(D);
   return true;
@@ -497,7 +497,7 @@ void ForLoopIndexUseVisitor::addUsage(const Usage &U) {
 ///     int k = *i + 2;
 ///   }
 /// \endcode
-bool ForLoopIndexUseVisitor::TraverseUnaryOperator(UnaryOperator *Uop) {
+bool ForLoopIndexUseVisitor::TraverseUnaryOperator(const UnaryOperator *Uop) {
   // If we dereference an iterator that's actually a pointer, count the
   // occurrence.
   if (isDereferenceOfUop(Uop, IndexVar)) {
@@ -533,7 +533,7 @@ bool ForLoopIndexUseVisitor::TraverseUnaryOperator(UnaryOperator *Uop) {
 ///   }
 /// \endcode
 /// will not.
-bool ForLoopIndexUseVisitor::TraverseMemberExpr(MemberExpr *Member) {
+bool ForLoopIndexUseVisitor::TraverseMemberExpr(const MemberExpr *Member) {
   const Expr *Base = Member->getBase();
   const DeclRefExpr *Obj = getDeclRef(Base);
   const Expr *ResultExpr = Member;
@@ -593,8 +593,8 @@ bool ForLoopIndexUseVisitor::TraverseMemberExpr(MemberExpr *Member) {
 /// Calls on the iterator object are not permitted, unless done through
 /// operator->(). The one exception is allowing vector::at() for pseudoarrays.
 bool ForLoopIndexUseVisitor::TraverseCXXMemberCallExpr(
-    CXXMemberCallExpr *MemberCall) {
-  auto *Member =
+    const CXXMemberCallExpr *MemberCall) {
+  const auto *Member =
       dyn_cast<MemberExpr>(MemberCall->getCallee()->IgnoreParenImpCasts());
   if (!Member)
     return VisitorBase::TraverseCXXMemberCallExpr(MemberCall);
@@ -638,7 +638,7 @@ bool ForLoopIndexUseVisitor::TraverseCXXMemberCallExpr(
 ///   }
 /// \endcode
 bool ForLoopIndexUseVisitor::TraverseCXXOperatorCallExpr(
-    CXXOperatorCallExpr *OpCall) {
+    const CXXOperatorCallExpr *OpCall) {
   switch (OpCall->getOperator()) {
   case OO_Star:
     if (isDereferenceOfOpCall(OpCall, IndexVar)) {
@@ -683,8 +683,9 @@ bool ForLoopIndexUseVisitor::TraverseCXXOperatorCallExpr(
 /// \endcode
 /// and further checking needs to be done later to ensure that exactly one array
 /// is referenced.
-bool ForLoopIndexUseVisitor::TraverseArraySubscriptExpr(ArraySubscriptExpr *E) {
-  Expr *Arr = E->getBase();
+bool ForLoopIndexUseVisitor::TraverseArraySubscriptExpr(
+    const ArraySubscriptExpr *E) {
+  const Expr *Arr = E->getBase();
   if (!isIndexInSubscriptExpr(E->getIdx(), IndexVar))
     return VisitorBase::TraverseArraySubscriptExpr(E);
 
@@ -737,7 +738,7 @@ bool ForLoopIndexUseVisitor::TraverseArraySubscriptExpr(ArraySubscriptExpr *E) {
 ///    for (int i = 0; i < obj.getVector().size(); ++i)
 ///      obj.foo(10); // using `obj` is considered risky
 /// \endcode
-bool ForLoopIndexUseVisitor::VisitDeclRefExpr(DeclRefExpr *E) {
+bool ForLoopIndexUseVisitor::VisitDeclRefExpr(const DeclRefExpr *E) {
   const ValueDecl *TheDecl = E->getDecl();
   if (areSameVariable(IndexVar, TheDecl) ||
       exprReferencesVariable(IndexVar, E) || areSameVariable(EndVar, TheDecl) ||
@@ -770,9 +771,9 @@ bool ForLoopIndexUseVisitor::VisitDeclRefExpr(DeclRefExpr *E) {
 ///     f(elem);
 ///   }
 /// \endcode
-bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
+bool ForLoopIndexUseVisitor::TraverseLambdaCapture(const LambdaExpr *LE,
                                                    const LambdaCapture *C,
-                                                   Expr *Init) {
+                                                   const Expr *Init) {
   if (C->capturesVariable()) {
     ValueDecl *VDecl = C->getCapturedVar();
     if (areSameVariable(IndexVar, VDecl)) {
@@ -785,7 +786,7 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
                      C->getLocation()));
     }
     if (VDecl->isInitCapture())
-      TraverseStmtImpl(cast<VarDecl>(VDecl)->getInit());
+      traverseStmtImpl(cast<VarDecl>(VDecl)->getInit());
   }
   return VisitorBase::TraverseLambdaCapture(LE, C, Init);
 }
@@ -794,7 +795,7 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
 /// element, note it for reuse as the loop variable.
 ///
 /// See the comments for isAliasDecl.
-bool ForLoopIndexUseVisitor::VisitDeclStmt(DeclStmt *S) {
+bool ForLoopIndexUseVisitor::VisitDeclStmt(const DeclStmt *S) {
   if (!AliasDecl && S->isSingleDecl() &&
       isAliasDecl(Context, S->getSingleDecl(), IndexVar)) {
     AliasDecl = S;
@@ -815,7 +816,7 @@ bool ForLoopIndexUseVisitor::VisitDeclStmt(DeclStmt *S) {
   return true;
 }
 
-bool ForLoopIndexUseVisitor::TraverseStmtImpl(Stmt *S) {
+bool ForLoopIndexUseVisitor::traverseStmtImpl(const Stmt *S) {
   // All this pointer swapping is a mechanism for tracking immediate parentage
   // of Stmts.
   const Stmt *OldNextParent = NextStmtParent;
@@ -826,7 +827,7 @@ bool ForLoopIndexUseVisitor::TraverseStmtImpl(Stmt *S) {
   return Result;
 }
 
-bool ForLoopIndexUseVisitor::TraverseStmt(Stmt *S) {
+bool ForLoopIndexUseVisitor::TraverseStmt(const Stmt *S) {
   // If this is an initialization expression for a lambda capture, prune the
   // traversal so that we don't end up diagnosing the contained DeclRefExpr as
   // inconsistent usage. No need to record the usage here -- this is done in
@@ -838,7 +839,7 @@ bool ForLoopIndexUseVisitor::TraverseStmt(Stmt *S) {
       return true;
     }
   }
-  return TraverseStmtImpl(S);
+  return traverseStmtImpl(S);
 }
 
 std::string VariableNamer::createIndexName() {
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index 306eca7140d1a..dc7ab847d9921 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_LOOP_CONVERT_UTILS_H
 
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseMap.h"
@@ -53,8 +53,7 @@ using ComponentVector = llvm::SmallVector<const clang::Expr *, 16>;
 
 /// Class used build the reverse AST properties needed to detect
 /// name conflicts and free variables.
-class StmtAncestorASTVisitor
-    : public clang::RecursiveASTVisitor<StmtAncestorASTVisitor> {
+class StmtAncestorASTVisitor : public ConstDynamicRecursiveASTVisitor {
 public:
   StmtAncestorASTVisitor() { StmtStack.push_back(nullptr); }
 
@@ -72,45 +71,37 @@ class StmtAncestorASTVisitor
   /// Accessor for DeclParents.
   const DeclParentMap &getDeclToParentStmtMap() { return DeclParents; }
 
-  friend class clang::RecursiveASTVisitor<StmtAncestorASTVisitor>;
-
 private:
   StmtParentMap StmtAncestors;
   DeclParentMap DeclParents;
   llvm::SmallVector<const clang::Stmt *, 16> StmtStack;
 
-  bool TraverseStmt(clang::Stmt *Statement);
-  bool VisitDeclStmt(clang::DeclStmt *Statement);
+  bool TraverseStmt(const Stmt *Statement) override;
+  bool VisitDeclStmt(const DeclStmt *Statement) override;
 };
 
 /// Class used to find the variables and member expressions on which an
 /// arbitrary expression depends.
-class ComponentFinderASTVisitor
-    : public clang::RecursiveASTVisitor<ComponentFinderASTVisitor> {
+class ComponentFinderASTVisitor : public ConstDynamicRecursiveASTVisitor {
 public:
   ComponentFinderASTVisitor() = default;
 
   /// Find the...
[truncated]

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all seems really mechanical as well, so I dont' have any concerns with that. However, the compile-time-tracker IS a bit concerning. The DRAV vs RAV decision is very much a performance-vs-size debate, and I don't like that we're giving up a percent+ on a few of these benchmarks (between this and the last one) in exchange for this size.

What I would LIKE to see is some analysis of:
1- How many instantiations these have
-Thus: How much space taken, and saved by switch
2- Which are hot-path enough to affect benchmarks, and which are irrelevant

For example, the tooling, unittests, tidy/etc variants seem like an easy decision for me, even if the answer to 1 above is "only 1 instantiation".

However, the ones in CodeGen or AST, or Debug info, etc, I'd want to see a significant savings, since those clearly affect compile time.

@Sirraide
Copy link
Member Author

1- How many instantiations these have
-Thus: How much space taken, and saved by switch

For the ones outside clang-tools-extra, none are in headers so they just replace one instantiation each.

2- Which are hot-path enough to affect benchmarks, and which are irrelevant

The conclusion we arrived at last time is that the performance is the same, and the regressions are beacuse more vtables means more relocations; last time I tested that by migrating only the unexpanded parameter packs visitor and observed no difference in performance, so I would assume this is a similar situation, but I’ll revert this change for some of the visitors in codegen and see if this changes anything.

@erichkeane
Copy link
Collaborator

1- How many instantiations these have
-Thus: How much space taken, and saved by switch

For the ones outside clang-tools-extra, none are in headers so they just replace one instantiation each.

2- Which are hot-path enough to affect benchmarks, and which are irrelevant

The conclusion we arrived at last time is that the performance is the same, and the regressions are beacuse more vtables means more relocations; last time I tested that by migrating only the unexpanded parameter packs visitor and observed no difference in performance, so I would assume this is a similar situation, but I’ll revert this change for some of the visitors in codegen and see if this changes anything.

Yeah, sorry, I'm not caught up on this (and missed it last time through). Aaron is out another week or two, so it might be better to wait and see what he has to say/his thoughts here.

@Sirraide Sirraide requested a review from nikic September 23, 2025 13:42
@cor3ntin
Copy link
Contributor

Looking at the benchmarks, are we making everything slower to improve the built times of clang itself? This seems like the wrong trade-off to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:static analyzer clang Clang issues not falling into any other category clang-tidy clang-tools-extra clangd debuginfo HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants